Skip to content

feat(xmldsig): add KeyInfo dispatch parsing#47

Merged
polaz merged 21 commits intomainfrom
feat/#46-keyinfo-dispatch
Apr 9, 2026
Merged

feat(xmldsig): add KeyInfo dispatch parsing#47
polaz merged 21 commits intomainfrom
feat/#46-keyinfo-dispatch

Conversation

@polaz
Copy link
Copy Markdown
Member

@polaz polaz commented Apr 4, 2026

Summary

  • implement ROADMAP P2-001 KeyInfo child dispatch parsing
  • support KeyName, KeyValue, X509Data, and dsig11 DEREncodedKeyValue sources
  • wire conditional KeyInfo parsing into the verify pipeline: explicit-key paths skip it, while keyless flows parse it (resolver-gated)

Details

  • added parse types and parser in src/xmldsig/parse.rs
  • re-exported new KeyInfo parsing API from src/xmldsig/mod.rs
  • unknown top-level <KeyInfo> children are ignored in lax mode
  • unknown XMLDSig-owned children (ds:* / dsig11:*) inside <X509Data> are rejected fail-closed
  • empty/extension-only <X509Data> remains accepted (lax), while malformed XMLDSig-owned children are rejected
  • ECKeyValue dispatch is namespace-strict: only dsig11:ECKeyValue is treated as supported, while ds:ECKeyValue is preserved as unsupported
  • added pipeline and parser tests in parse.rs and verify.rs

Validation

  • cargo fmt -- --check
  • cargo clippy --all-targets --all-features -- -D warnings
  • cargo nextest run --workspace
  • cargo test --doc --workspace

Closes #46

- parse KeyInfo child sources: KeyName, KeyValue, X509Data, DEREncodedKeyValue
- wire KeyInfo parsing into signature verification pipeline with explicit ParseKeyInfo error
- add parser and pipeline coverage tests for order and malformed input

Closes #46
Copilot AI review requested due to automatic review settings April 4, 2026 20:44
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 4, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8a67b0ea-fbc2-41ad-93b0-6df0bacb4d00

📥 Commits

Reviewing files that changed from the base of the PR and between dad900e and 204cf57.

📒 Files selected for processing (4)
  • src/xmldsig/mod.rs
  • src/xmldsig/parse.rs
  • src/xmldsig/verify.rs
  • src/xmldsig/whitespace.rs

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Added support for XMLDSig 1.1 standard
    • Enabled parsing and validation of key information elements, including support for DER-encoded keys
    • Enhanced signature verification with stricter structural validation
  • Improvements

    • Improved base64 content handling with normalized whitespace processing
    • Added structural constraints for key information parsing

Walkthrough

Adds KeyInfo parsing (KeyName, KeyValue, X509Data, dsig11:DEREncodedKeyValue) and exported KeyInfo models; factors XML whitespace/base64 normalization into a crate-visible helper; wires conditional KeyInfo parsing into verification and tightens child-order and mixed-content validation.

Changes

Cohort / File(s) Summary
KeyInfo Parser & Models
src/xmldsig/parse.rs
Adds XMLDSIG 1.1 namespace constant, new exported types (KeyInfo, KeyInfoSource, KeyValueInfo, X509DataInfo), and pub fn parse_key_info(Node) -> Result<KeyInfo, ParseError) with ordered-child dispatch, strict structural and base64/DER/size validation, refactors digest base64 handling, and adds unit tests.
Signature Verification Flow
src/xmldsig/verify.rs
Extends KeyResolver with consumes_document_key_info(), conditions when parse_key_info is invoked, maps parse failures to DsigError::ParseKeyInfo, enforces element ordering and rejects non-whitespace mixed text/foreign-namespace children under <Signature>, and normalizes SignatureValue base64 via helper.
Whitespace Helper & Module Surface
src/xmldsig/whitespace.rs, src/xmldsig/mod.rs
Adds pub(crate) whitespace module with is_xml_whitespace_only, normalize_xml_base64_text, and XmlBase64NormalizeError; updates mod.rs to pub(crate) mod whitespace; and re-exports new parse symbols (KeyInfo, KeyInfoSource, KeyValueInfo, X509DataInfo, parse_key_info).

Sequence Diagram(s)

sequenceDiagram
    autonumber
    actor Caller
    participant Verify as "verify.rs"
    participant ParseSigned as "parse.rs (parse_signed_info)"
    participant ParseKey as "parse.rs (parse_key_info)"
    participant Whitespace as "whitespace.rs"

    Caller->>Verify: verify_signature_with_context(signature_node, ctx)
    Verify->>ParseSigned: parse_signed_info(signed_info_node)
    ParseSigned-->>Verify: SignedInfo
    Verify->>Verify: parse_signature_value()
    alt KeyInfo present or resolver.consumes_document_key_info() == true
        Verify->>ParseKey: parse_key_info(key_info_node)
        ParseKey->>Whitespace: normalize_xml_base64_text / is_xml_whitespace_only
        ParseKey->>ParseKey: dispatch children (KeyName, KeyValue, X509Data, DEREncodedKeyValue)
        ParseKey-->>Verify: KeyInfo or ParseError
    else KeyInfo absent / ignored
        Note over Verify: continue without KeyInfo
    end
    Verify-->>Caller: verification result or DsigError::ParseKeyInfo
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding KeyInfo dispatch parsing functionality to the xmldsig module.
Description check ✅ Passed The description is well-organized and covers the implementation details, testing, and validation. It relates to the changeset and provides meaningful context.
Linked Issues check ✅ Passed The PR successfully implements all acceptance criteria from issue #46: KeyInfo parsing dispatch for KeyName/KeyValue/X509Data/DEREncodedKeyValue, backward compatibility, and full workspace checks.
Out of Scope Changes check ✅ Passed All changes directly support KeyInfo dispatch parsing and verify pipeline integration as specified in issue #46; sha1/sha2 upgrade is explicitly noted as deferred/out-of-scope.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/#46-keyinfo-dispatch

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds initial XMLDSig <KeyInfo> child dispatch parsing (ROADMAP P2-001) and wires it into the verification pipeline so malformed/unsupported structures can be surfaced as explicit pipeline parse errors.

Changes:

  • Introduces KeyInfo/KeyInfoSource parsing with dispatch for KeyName, KeyValue, X509Data, and dsig11:DEREncodedKeyValue.
  • Integrates <KeyInfo> parsing into the verify pipeline and enforces <Signature> child ordering with optional KeyInfo as the 3rd element child.
  • Re-exports the new parsing API from the xmldsig module and adds targeted unit tests for parser + pipeline behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/xmldsig/parse.rs Adds KeyInfo model types and a dispatch parser for supported KeyInfo child elements + unit tests.
src/xmldsig/verify.rs Parses optional KeyInfo in the verify pipeline, adds ParseKeyInfo error mapping, and enforces Signature child ordering + pipeline tests.
src/xmldsig/mod.rs Re-exports new KeyInfo parsing types/functions as part of the public API.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/xmldsig/verify.rs (1)

964-1043: ⚠️ Potential issue | 🟠 Major

Reject unexpected <Signature> children after the allowed prefix.

This index tracking only proves where SignedInfo, SignatureValue, and KeyInfo appear; it still accepts "<Signature><SignedInfo/><SignatureValue/><Foo/></Signature>" and "...<KeyInfo/><Foo/>" because any other trailing element is ignored. The content model after the optional KeyInfo is only Object*, so this should fail closed and get a regression test.

As per coding guidelines, Tier 1 - Flag logic bugs: missing validation (unvalidated input from network/disk/external source).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/xmldsig/verify.rs` around lines 964 - 1043, The parser currently ignores
unexpected XMLDSIG element children after the allowed prefix, so
"<Signature><SignedInfo/><SignatureValue/><Foo/></Signature>" wrongly succeeds;
fix parse_signature_children to count only XMLDSIG element children (use a
xml_element_index counter incremented only when child.tag_name().namespace() ==
Some(XMLDSIG_NS)) and enforce the content model: require SignedInfo at xml index
1 and SignatureValue at 2, allow optional KeyInfo at 3, and permit only "Object"
elements after that; treat any other XMLDSIG element name at any position as
SignatureVerificationPipelineError::InvalidStructure (include the element name
in the reason) and fail if an unexpected element appears after the allowed
prefix. Reference: parse_signature_children, signed_info_index,
signature_value_index, key_info_index, and SignatureChildNodes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/xmldsig/parse.rs`:
- Around line 357-383: The loop over element_children(key_info_node) accepts
malformed mixed-content like "<KeyValue>junk<RSAKeyValue/></KeyValue>" because
element_children() and collect_text_content() silently drop unexpected node
kinds; update the KeyInfo parsing to explicitly reject non-whitespace mixed
content and unexpected child elements: for KeyName (in the KeyName arm) assert
the node contains no element children and only text (trim() non-empty allowed,
any embedded element nodes or non-whitespace text before/after elements =>
return Err(ParseError::InvalidStructure(...))); for KeyValue and X509Data ensure
the node has no non-whitespace text nodes mixed with element children before
calling parse_key_value_dispatch or parse_x509_data_dispatch; for
DEREncodedKeyValue ensure there are no element children and the text node is the
only content before calling decode_base64_xml_text; return a clear
ParseError::InvalidStructure on violations. Apply the same validation pattern to
the other parsing locations you mentioned (around the
parse_key_value_dispatch/parse_x509_data_dispatch and DER decoding sites).
- Around line 115-161: Add forward-compatibility by marking the public parse
model as non-exhaustive and tighten field visibility: add #[non_exhaustive] to
KeyInfo, KeyInfoSource, KeyValueInfo, and X509DataInfo; change internal fields
that need not be publicly mutable (KeyInfo.sources and the X509DataInfo *_count
fields) from pub to pub(crate) so consumers cannot match exhaustively or rely on
concrete variants/fields; leave enum variants and public types exported but rely
on non_exhaustive to allow adding new variants/fields in future.

---

Outside diff comments:
In `@src/xmldsig/verify.rs`:
- Around line 964-1043: The parser currently ignores unexpected XMLDSIG element
children after the allowed prefix, so
"<Signature><SignedInfo/><SignatureValue/><Foo/></Signature>" wrongly succeeds;
fix parse_signature_children to count only XMLDSIG element children (use a
xml_element_index counter incremented only when child.tag_name().namespace() ==
Some(XMLDSIG_NS)) and enforce the content model: require SignedInfo at xml index
1 and SignatureValue at 2, allow optional KeyInfo at 3, and permit only "Object"
elements after that; treat any other XMLDSIG element name at any position as
SignatureVerificationPipelineError::InvalidStructure (include the element name
in the reason) and fail if an unexpected element appears after the allowed
prefix. Reference: parse_signature_children, signed_info_index,
signature_value_index, key_info_index, and SignatureChildNodes.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: f04a3c91-55a7-4840-8c05-39edae4ff650

📥 Commits

Reviewing files that changed from the base of the PR and between dad900e and 7063ba0.

📒 Files selected for processing (3)
  • src/xmldsig/mod.rs
  • src/xmldsig/parse.rs
  • src/xmldsig/verify.rs

@polaz
Copy link
Copy Markdown
Member Author

polaz commented Apr 4, 2026

Addressed the outside-diff item as well in commit 2004cf8: now fails closed for unexpected XMLDSIG children after the allowed SignedInfo/SignatureValue/(optional KeyInfo) prefix, and the regression suite passes.

@polaz
Copy link
Copy Markdown
Member Author

polaz commented Apr 4, 2026

Correction: outside-diff item is fixed in commit 2004cf8. Function parse_signature_children now rejects unexpected XMLDSIG children after the allowed SignedInfo/SignatureValue/(optional KeyInfo) prefix, and full regression tests are green.

@polaz polaz requested a review from Copilot April 4, 2026 21:27
@polaz
Copy link
Copy Markdown
Member Author

polaz commented Apr 4, 2026

@coderabbitai re-review

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 4, 2026

@polaz: Thanks for the follow-up and for confirming that commit 2004cf8 hardens parse_signature_children to fail closed on unexpected XMLDSIG children after the allowed SignedInfo/SignatureValue/(optional KeyInfo) prefix. I'll kick off a fresh review of the PR now.


🧠 Learnings used
Learnt from: polaz
Repo: structured-world/xml-sec PR: 0
File: :0-0
Timestamp: 2026-03-28T15:50:07.840Z
Learning: In `src/xmldsig/verify.rs` (xml-sec Rust project), `push_normalized_signature_text` strips only the four XML whitespace characters (space 0x20, tab 0x09, CR 0x0D, LF 0x0A) from `SignatureValue` text before base64 decoding, and explicitly rejects any other ASCII whitespace (e.g., form feed 0x0C, vertical tab 0x0B) as `SignatureVerificationPipelineError::SignatureValueBase64(base64::DecodeError::InvalidByte(...))`. The regression test `push_normalized_signature_text_rejects_form_feed` covers the form-feed boundary case.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 32
File: src/xmldsig/verify.rs:34-51
Timestamp: 2026-03-28T22:00:29.444Z
Learning: In `src/xmldsig/verify.rs` (xml-sec Rust project), the `VerifyingKey` and `KeyResolver` public traits intentionally omit `Send + Sync` supertraits to preserve lightweight single-threaded implementations. The single-threaded-only constraint is documented on both trait definitions. Do not flag the absence of `Send + Sync` on these traits as a review issue.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 41
File: tests/donor_full_verification_suite.rs:165-178
Timestamp: 2026-04-04T06:14:34.735Z
Learning: In `tests/donor_full_verification_suite.rs` (xml-sec Rust project), the `Expectation::Skip` arm must exercise skipped vectors through `VerifyContext::new().verify(xml)` (no injected PEM key) so that the real key-resolution path runs, and must assert a concrete blocker outcome per case: `Invalid(KeyNotFound)` for vectors with unresolved KeyValue/X.509 key material, and `ParseSignedInfo(UnsupportedAlgorithm)` for DSA-signature vectors. Accepting any `Err` or non-Valid result as a valid skip is insufficient and can mask stale skip reasons or unrelated failures.
✅ Actions performed

Full review triggered.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

@polaz
Copy link
Copy Markdown
Member Author

polaz commented Apr 4, 2026

@coderabbitai Outside-diff item (verify.rs 964-1043) is already addressed: parse_signature_children now fails closed on unexpected XMLDSIG children after SignedInfo/SignatureValue/(optional KeyInfo), and workspace tests are green after latest commit 5238c92.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 4, 2026

Tip

For best results, initiate chat on the files or code changes.

@polaz: Confirmed — thanks for the pointer to commit 5238c92. With parse_signature_children now failing closed on any unexpected XMLDSIG-namespace children beyond the allowed SignedInfo / SignatureValue / (optional KeyInfo) prefix, and the full workspace suite green, the outside-diff concern is fully resolved.


🧠 Learnings used
Learnt from: polaz
Repo: structured-world/xml-sec PR: 0
File: :0-0
Timestamp: 2026-03-28T15:50:07.840Z
Learning: In `src/xmldsig/verify.rs` (xml-sec Rust project), `push_normalized_signature_text` strips only the four XML whitespace characters (space 0x20, tab 0x09, CR 0x0D, LF 0x0A) from `SignatureValue` text before base64 decoding, and explicitly rejects any other ASCII whitespace (e.g., form feed 0x0C, vertical tab 0x0B) as `SignatureVerificationPipelineError::SignatureValueBase64(base64::DecodeError::InvalidByte(...))`. The regression test `push_normalized_signature_text_rejects_form_feed` covers the form-feed boundary case.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 41
File: tests/donor_full_verification_suite.rs:165-178
Timestamp: 2026-04-04T06:14:34.735Z
Learning: In `tests/donor_full_verification_suite.rs` (xml-sec Rust project), the `Expectation::Skip` arm must exercise skipped vectors through `VerifyContext::new().verify(xml)` (no injected PEM key) so that the real key-resolution path runs, and must assert a concrete blocker outcome per case: `Invalid(KeyNotFound)` for vectors with unresolved KeyValue/X.509 key material, and `ParseSignedInfo(UnsupportedAlgorithm)` for DSA-signature vectors. Accepting any `Err` or non-Valid result as a valid skip is insufficient and can mask stale skip reasons or unrelated failures.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 0
File: :0-0
Timestamp: 2026-03-28T18:51:49.919Z
Learning: In `src/xmldsig/parse.rs` (xml-sec Rust project), the private function `n` (formerly `base64_decode_digest`) strips only the four XML whitespace characters (space 0x20, tab 0x09, CR 0x0D, LF 0x0A) from `DigestValue` base64 text before decoding, and returns `ParseError::Base64(format!("invalid XML whitespace U+{:04X} ...", ...))` for any other `is_ascii_whitespace()` character (e.g. form feed 0x0C). Regression tests `n_accepts_xml_whitespace_chars` and `n_rejects_non_xml_ascii_whitespace` cover these cases.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 32
File: src/xmldsig/verify.rs:34-51
Timestamp: 2026-03-28T22:00:29.444Z
Learning: In `src/xmldsig/verify.rs` (xml-sec Rust project), the `VerifyingKey` and `KeyResolver` public traits intentionally omit `Send + Sync` supertraits to preserve lightweight single-threaded implementations. The single-threaded-only constraint is documented on both trait definitions. Do not flag the absence of `Send + Sync` on these traits as a review issue.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 0
File: :0-0
Timestamp: 2026-03-28T20:47:05.798Z
Learning: In `src/xmldsig/verify.rs` (xml-sec Rust project), the ASCII-whitespace branch in `push_normalized_signature_text` uses `u8::try_from(u32::from(ch)).expect("ASCII whitespace always fits into u8")` for the explicit narrowing conversion, producing `base64::DecodeError::InvalidByte(normalized.len(), invalid_byte)` as the error payload. The conversion is provably infallible because `ch.is_ascii_whitespace()` guarantees the character is in the 0x00–0x7F range.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 12
File: src/c14n/serialize.rs:382-398
Timestamp: 2026-03-16T17:41:41.053Z
Learning: Repo: structured-world/xml-sec (Rust)
Context: src/c14n/serialize.rs::collect_inherited_xml_attrs
Learning: For inclusive C14N subsets, collect_inherited_xml_attrs must stop at the nearest included ancestor (A in set → B out → C in) to avoid inheriting xml:* through an included ancestor. A unit test named `no_inheritance_past_included_ancestor` covers this scenario. Exclusive C14N should skip this inheritance step entirely.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 12
File: src/c14n/xml_base.rs:61-93
Timestamp: 2026-03-16T17:23:26.715Z
Learning: In the xml-sec Rust project, the xml:base URI resolver in `src/c14n/xml_base.rs` correctly uses RFC 3986 §5 for IRI resolution in C14N 1.1 mode, as C14N 1.1 §2.4 explicitly delegates to RFC 3986 §5. When the base URI has no scheme, `parse_base()` returns `None` and the reference is returned as-is — this is the correct fallback behavior. The W3C C14N 1.1 interop test suite notes "modifications to RFC 3986 dot segment processing for C14N11" which may represent edge cases worth verifying against the `remove_dot_segments()` implementation.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 16
File: tests/fixtures_smoke.rs:104-104
Timestamp: 2026-03-17T15:17:20.125Z
Learning: In structured-world/xml-sec (Rust), `expect()` on `fs::read_to_string()` and other I/O calls is acceptable and idiomatic inside `#[test]` functions. The Tier 2 rule (use `Result` propagation instead of `expect()` on I/O paths) applies only to production code paths, not test code. Repo instructions (line 51) state: "Test code style: Tests prioritize readability and explicitness over DRY."

Learnt from: polaz
Repo: structured-world/xml-sec PR: 6
File: src/c14n/ns_exclusive.rs:81-107
Timestamp: 2026-03-15T19:26:11.246Z
Learning: In the xml-sec project (Rust), roxmltree 0.21 does not expose lexical prefixes from parsed QNames — `ExpandedName` only provides `namespace()` and `name()`. As a result, `visibly_utilized_prefixes()` in `src/c14n/ns_exclusive.rs` and `write_qualified_name()` in `src/c14n/serialize.rs` must use `lookup_prefix(namespace_uri)` for prefix resolution. This is a known limitation: when multiple prefixes bind the same namespace URI, the reverse-mapping is ambiguous. The ambiguity is documented in code comments and tracked as a ROADMAP gap; the edge case is considered rare in SAML/XMLDSig use cases.

Learnt from: CR
Repo: structured-world/xml-sec PR: 0
File: .github/instructions/rust.instructions.md:0-0
Timestamp: 2026-03-17T12:12:28.146Z
Learning: Review ONLY code within the PR's diff; for issues outside the diff, suggest opening a separate issue

Learnt from: CR
Repo: structured-world/xml-sec PR: 0
File: .github/instructions/rust.instructions.md:0-0
Timestamp: 2026-03-17T12:12:28.146Z
Learning: Applies to **/*.rs : Tier 1 - Flag logic bugs: checksum/hash mismatches (computing over wrong byte range, verifying against stale value)

Learnt from: CR
Repo: structured-world/xml-sec PR: 0
File: .github/instructions/code-review.instructions.md:0-0
Timestamp: 2026-03-17T12:12:07.790Z
Learning: Applies to **/*.rs : Corruption/validation tests: tamper the relevant field (e.g., digest value, signature bytes, base64 encoding) and assert the error

@polaz polaz requested a review from Copilot April 4, 2026 22:48
@polaz
Copy link
Copy Markdown
Member Author

polaz commented Apr 4, 2026

@coderabbitai re-review

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 4, 2026

@polaz: Kicking off a fresh review of PR #47 now. I'll examine all the latest changes including the parse_key_info dispatcher, the KeyInfo/KeyInfoSource/KeyValueInfo/X509DataInfo data model, the pipeline wiring in verify.rs, and the hardened parse_signature_children from commits 2004cf8/5238c92.


🧠 Learnings used
Learnt from: polaz
Repo: structured-world/xml-sec PR: 0
File: :0-0
Timestamp: 2026-03-28T15:50:07.840Z
Learning: In `src/xmldsig/verify.rs` (xml-sec Rust project), `push_normalized_signature_text` strips only the four XML whitespace characters (space 0x20, tab 0x09, CR 0x0D, LF 0x0A) from `SignatureValue` text before base64 decoding, and explicitly rejects any other ASCII whitespace (e.g., form feed 0x0C, vertical tab 0x0B) as `SignatureVerificationPipelineError::SignatureValueBase64(base64::DecodeError::InvalidByte(...))`. The regression test `push_normalized_signature_text_rejects_form_feed` covers the form-feed boundary case.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 41
File: tests/donor_full_verification_suite.rs:165-178
Timestamp: 2026-04-04T06:14:34.735Z
Learning: In `tests/donor_full_verification_suite.rs` (xml-sec Rust project), the `Expectation::Skip` arm must exercise skipped vectors through `VerifyContext::new().verify(xml)` (no injected PEM key) so that the real key-resolution path runs, and must assert a concrete blocker outcome per case: `Invalid(KeyNotFound)` for vectors with unresolved KeyValue/X.509 key material, and `ParseSignedInfo(UnsupportedAlgorithm)` for DSA-signature vectors. Accepting any `Err` or non-Valid result as a valid skip is insufficient and can mask stale skip reasons or unrelated failures.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 32
File: src/xmldsig/verify.rs:34-51
Timestamp: 2026-03-28T22:00:29.444Z
Learning: In `src/xmldsig/verify.rs` (xml-sec Rust project), the `VerifyingKey` and `KeyResolver` public traits intentionally omit `Send + Sync` supertraits to preserve lightweight single-threaded implementations. The single-threaded-only constraint is documented on both trait definitions. Do not flag the absence of `Send + Sync` on these traits as a review issue.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 0
File: :0-0
Timestamp: 2026-03-28T18:51:49.919Z
Learning: In `src/xmldsig/parse.rs` (xml-sec Rust project), the private function `n` (formerly `base64_decode_digest`) strips only the four XML whitespace characters (space 0x20, tab 0x09, CR 0x0D, LF 0x0A) from `DigestValue` base64 text before decoding, and returns `ParseError::Base64(format!("invalid XML whitespace U+{:04X} ...", ...))` for any other `is_ascii_whitespace()` character (e.g. form feed 0x0C). Regression tests `n_accepts_xml_whitespace_chars` and `n_rejects_non_xml_ascii_whitespace` cover these cases.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 0
File: :0-0
Timestamp: 2026-03-28T20:47:05.798Z
Learning: In `src/xmldsig/verify.rs` (xml-sec Rust project), the ASCII-whitespace branch in `push_normalized_signature_text` uses `u8::try_from(u32::from(ch)).expect("ASCII whitespace always fits into u8")` for the explicit narrowing conversion, producing `base64::DecodeError::InvalidByte(normalized.len(), invalid_byte)` as the error payload. The conversion is provably infallible because `ch.is_ascii_whitespace()` guarantees the character is in the 0x00–0x7F range.

Learnt from: CR
Repo: structured-world/xml-sec PR: 0
File: .github/instructions/rust.instructions.md:0-0
Timestamp: 2026-03-17T12:12:28.146Z
Learning: Applies to **/*.rs : Tier 1 - Flag logic bugs: checksum/hash mismatches (computing over wrong byte range, verifying against stale value)

Learnt from: polaz
Repo: structured-world/xml-sec PR: 16
File: tests/fixtures_smoke.rs:104-104
Timestamp: 2026-03-17T15:17:20.125Z
Learning: In structured-world/xml-sec (Rust), `expect()` on `fs::read_to_string()` and other I/O calls is acceptable and idiomatic inside `#[test]` functions. The Tier 2 rule (use `Result` propagation instead of `expect()` on I/O paths) applies only to production code paths, not test code. Repo instructions (line 51) state: "Test code style: Tests prioritize readability and explicitness over DRY."

Learnt from: CR
Repo: structured-world/xml-sec PR: 0
File: .github/instructions/code-review.instructions.md:0-0
Timestamp: 2026-03-17T12:12:07.790Z
Learning: Applies to **/*.rs : Corruption/validation tests: tamper the relevant field (e.g., digest value, signature bytes, base64 encoding) and assert the error

Learnt from: CR
Repo: structured-world/xml-sec PR: 0
File: .github/instructions/rust.instructions.md:0-0
Timestamp: 2026-03-17T12:12:28.146Z
Learning: Applies to **/*.rs : Tier 2 - Flag safety issues: constant-time comparison must be used for cryptographic MACs/checksums

Learnt from: polaz
Repo: structured-world/xml-sec PR: 12
File: src/c14n/serialize.rs:382-398
Timestamp: 2026-03-16T17:41:41.053Z
Learning: Repo: structured-world/xml-sec (Rust)
Context: src/c14n/serialize.rs::collect_inherited_xml_attrs
Learning: For inclusive C14N subsets, collect_inherited_xml_attrs must stop at the nearest included ancestor (A in set → B out → C in) to avoid inheriting xml:* through an included ancestor. A unit test named `no_inheritance_past_included_ancestor` covers this scenario. Exclusive C14N should skip this inheritance step entirely.
✅ Actions performed

Full review triggered.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/xmldsig/verify.rs (1)

976-984: ⚠️ Potential issue | 🟠 Major

Reject foreign and mixed-content children under <Signature>.

This scan only tracks XMLDSig element children and silently skips everything else, so malformed inputs like <foo:Bar/> or stray non-whitespace text after the allowed prefix still pass the new suffix checks. If this function is meant to enforce the SignedInfo, SignatureValue, optional KeyInfo, then Object* shape, it needs to fail closed on any non-XMLDSig element child and any non-whitespace mixed content under <Signature>.

🔒 Suggested hardening
-    for (zero_based_index, child) in signature_node
-        .children()
-        .filter(|node| node.is_element())
-        .enumerate()
-    {
-        let element_index = zero_based_index + 1;
-        if child.tag_name().namespace() != Some(XMLDSIG_NS) {
-            continue;
-        }
+    let mut element_index = 0usize;
+    for child in signature_node.children() {
+        if child.is_text()
+            && child.text().is_some_and(|text| {
+                text.chars().any(|c| !matches!(c, ' ' | '\t' | '\n' | '\r'))
+            })
+        {
+            return Err(SignatureVerificationPipelineError::InvalidStructure {
+                reason: "Signature must not contain non-whitespace mixed content",
+            });
+        }
+        if !child.is_element() {
+            continue;
+        }
+        element_index += 1;
+        if child.tag_name().namespace() != Some(XMLDSIG_NS) {
+            return Err(SignatureVerificationPipelineError::InvalidStructure {
+                reason: "Signature must contain only ds:SignedInfo, ds:SignatureValue, optional ds:KeyInfo, and ds:Object element children",
+            });
+        }

As per coding guidelines, Tier 1 - Flag logic bugs: missing validation (unchecked index, unvalidated input from network/disk/external source).

Also applies to: 1050-1072

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/xmldsig/verify.rs` around lines 976 - 984, The loop over signature_node
children in verify.rs currently ignores non-XMLDSIG elements and mixed content;
change it to fail-closed: inside the iteration over signature_node.children()
(the block around the for ... in signature_node.children().filter(|node|
node.is_element()).enumerate()), remove the filter that hides non-element nodes
and instead inspect every child node; if a child is an element and
child.tag_name().namespace() != Some(XMLDSIG_NS) return an error (reject foreign
elements), and if a child is not an element but contains non-whitespace text or
non-trivial nodes (comments/PIs as disallowed per spec) return an error (reject
mixed content); apply the same stricter checks in the corresponding later block
referenced at lines ~1050-1072 so any unexpected non-XMLDSIG element or
non-whitespace text under <Signature> causes verification to fail.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/xmldsig/parse.rs`:
- Around line 381-384: The DEREncodedKeyValue handler currently calls
collect_text_content(child) which clones the entire text payload before
decode_base64_xml_text enforces MAX_DER_ENCODED_KEY_VALUE_TEXT_LEN /
MAX_DER_ENCODED_KEY_VALUE_LEN; change this to stream/validate the text nodes
first by replacing collect_text_content+decode_base64_xml_text with a new
decode_base64_xml_children(child, ...) helper that iterates
child.children().filter(|n| n.is_text()), enforces the existing
length/whitespace caps (MAX_DER_ENCODED_KEY_VALUE_TEXT_LEN and
MAX_DER_ENCODED_KEY_VALUE_LEN) as it appends, and returns the decoded bytes so
you can still push KeyInfoSource::DerEncodedKeyValue(der); keep
ensure_no_element_children(child, "DEREncodedKeyValue") and reuse the same
validation logic elsewhere (e.g., the ranges noted at lines ~550-594).

In `@src/xmldsig/verify.rs`:
- Around line 2510-2529: The test pipeline_reports_keyinfo_parse_error currently
corrupts KeyInfo but leaves <SignedInfo/> invalid, making the failure dependent
on parser order; update the test fixture so <SignedInfo> is a minimally valid
SignedInfo (e.g., include CanonicalizationMethod, SignatureMethod and at least
one Reference with DigestMethod and a DigestValue) and keep only the
dsig11:DEREncodedKeyValue corrupted, then call verify_signature_with_pem_key and
assert the returned error is SignatureVerificationPipelineError::ParseKeyInfo(_)
to make the regression independent of element parse order.

---

Outside diff comments:
In `@src/xmldsig/verify.rs`:
- Around line 976-984: The loop over signature_node children in verify.rs
currently ignores non-XMLDSIG elements and mixed content; change it to
fail-closed: inside the iteration over signature_node.children() (the block
around the for ... in signature_node.children().filter(|node|
node.is_element()).enumerate()), remove the filter that hides non-element nodes
and instead inspect every child node; if a child is an element and
child.tag_name().namespace() != Some(XMLDSIG_NS) return an error (reject foreign
elements), and if a child is not an element but contains non-whitespace text or
non-trivial nodes (comments/PIs as disallowed per spec) return an error (reject
mixed content); apply the same stricter checks in the corresponding later block
referenced at lines ~1050-1072 so any unexpected non-XMLDSIG element or
non-whitespace text under <Signature> causes verification to fail.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 20e18548-9962-42e0-aaa9-80cd3a6c2bc8

📥 Commits

Reviewing files that changed from the base of the PR and between dad900e and 5238c92.

📒 Files selected for processing (3)
  • src/xmldsig/mod.rs
  • src/xmldsig/parse.rs
  • src/xmldsig/verify.rs

@polaz
Copy link
Copy Markdown
Member Author

polaz commented Apr 4, 2026

@coderabbitai Outside-diff item (verify.rs 976-984 / 1050-1072) fixed in eec05eb: parse_signature_children now iterates all child nodes and fails closed on foreign element children, non-whitespace text mixed content, and non-element children under .

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 4, 2026

Tip

For best results, initiate chat on the files or code changes.

@polaz: Confirmed — commit eec05eb completes the hardening of parse_signature_children. The three additional failure-closed paths (foreign-namespace element children, non-whitespace text mixed content, and non-element node types such as PIs or CDATA under <Signature>) close the remaining gaps beyond the XMLDSIG-namespace-only guard from 5238c92. The outside-diff concern is fully resolved.


🧠 Learnings used
Learnt from: polaz
Repo: structured-world/xml-sec PR: 0
File: :0-0
Timestamp: 2026-03-28T15:50:07.840Z
Learning: In `src/xmldsig/verify.rs` (xml-sec Rust project), `push_normalized_signature_text` strips only the four XML whitespace characters (space 0x20, tab 0x09, CR 0x0D, LF 0x0A) from `SignatureValue` text before base64 decoding, and explicitly rejects any other ASCII whitespace (e.g., form feed 0x0C, vertical tab 0x0B) as `SignatureVerificationPipelineError::SignatureValueBase64(base64::DecodeError::InvalidByte(...))`. The regression test `push_normalized_signature_text_rejects_form_feed` covers the form-feed boundary case.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 41
File: tests/donor_full_verification_suite.rs:165-178
Timestamp: 2026-04-04T06:14:34.735Z
Learning: In `tests/donor_full_verification_suite.rs` (xml-sec Rust project), the `Expectation::Skip` arm must exercise skipped vectors through `VerifyContext::new().verify(xml)` (no injected PEM key) so that the real key-resolution path runs, and must assert a concrete blocker outcome per case: `Invalid(KeyNotFound)` for vectors with unresolved KeyValue/X.509 key material, and `ParseSignedInfo(UnsupportedAlgorithm)` for DSA-signature vectors. Accepting any `Err` or non-Valid result as a valid skip is insufficient and can mask stale skip reasons or unrelated failures.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 32
File: src/xmldsig/verify.rs:34-51
Timestamp: 2026-03-28T22:00:29.444Z
Learning: In `src/xmldsig/verify.rs` (xml-sec Rust project), the `VerifyingKey` and `KeyResolver` public traits intentionally omit `Send + Sync` supertraits to preserve lightweight single-threaded implementations. The single-threaded-only constraint is documented on both trait definitions. Do not flag the absence of `Send + Sync` on these traits as a review issue.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 0
File: :0-0
Timestamp: 2026-03-28T18:51:49.919Z
Learning: In `src/xmldsig/parse.rs` (xml-sec Rust project), the private function `n` (formerly `base64_decode_digest`) strips only the four XML whitespace characters (space 0x20, tab 0x09, CR 0x0D, LF 0x0A) from `DigestValue` base64 text before decoding, and returns `ParseError::Base64(format!("invalid XML whitespace U+{:04X} ...", ...))` for any other `is_ascii_whitespace()` character (e.g. form feed 0x0C). Regression tests `n_accepts_xml_whitespace_chars` and `n_rejects_non_xml_ascii_whitespace` cover these cases.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 0
File: :0-0
Timestamp: 2026-03-28T20:47:05.798Z
Learning: In `src/xmldsig/verify.rs` (xml-sec Rust project), the ASCII-whitespace branch in `push_normalized_signature_text` uses `u8::try_from(u32::from(ch)).expect("ASCII whitespace always fits into u8")` for the explicit narrowing conversion, producing `base64::DecodeError::InvalidByte(normalized.len(), invalid_byte)` as the error payload. The conversion is provably infallible because `ch.is_ascii_whitespace()` guarantees the character is in the 0x00–0x7F range.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 12
File: src/c14n/serialize.rs:382-398
Timestamp: 2026-03-16T17:41:41.053Z
Learning: Repo: structured-world/xml-sec (Rust)
Context: src/c14n/serialize.rs::collect_inherited_xml_attrs
Learning: For inclusive C14N subsets, collect_inherited_xml_attrs must stop at the nearest included ancestor (A in set → B out → C in) to avoid inheriting xml:* through an included ancestor. A unit test named `no_inheritance_past_included_ancestor` covers this scenario. Exclusive C14N should skip this inheritance step entirely.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 12
File: src/c14n/xml_base.rs:61-93
Timestamp: 2026-03-16T17:23:26.715Z
Learning: In the xml-sec Rust project, the xml:base URI resolver in `src/c14n/xml_base.rs` correctly uses RFC 3986 §5 for IRI resolution in C14N 1.1 mode, as C14N 1.1 §2.4 explicitly delegates to RFC 3986 §5. When the base URI has no scheme, `parse_base()` returns `None` and the reference is returned as-is — this is the correct fallback behavior. The W3C C14N 1.1 interop test suite notes "modifications to RFC 3986 dot segment processing for C14N11" which may represent edge cases worth verifying against the `remove_dot_segments()` implementation.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 6
File: src/c14n/ns_exclusive.rs:81-107
Timestamp: 2026-03-15T19:26:11.246Z
Learning: In the xml-sec project (Rust), roxmltree 0.21 does not expose lexical prefixes from parsed QNames — `ExpandedName` only provides `namespace()` and `name()`. As a result, `visibly_utilized_prefixes()` in `src/c14n/ns_exclusive.rs` and `write_qualified_name()` in `src/c14n/serialize.rs` must use `lookup_prefix(namespace_uri)` for prefix resolution. This is a known limitation: when multiple prefixes bind the same namespace URI, the reverse-mapping is ambiguous. The ambiguity is documented in code comments and tracked as a ROADMAP gap; the edge case is considered rare in SAML/XMLDSig use cases.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 12
File: src/c14n/serialize.rs:218-222
Timestamp: 2026-03-16T18:07:55.027Z
Learning: In the xml-sec project (Rust), RFC 3741 §1.2 (Exclusive XML Canonicalization) explicitly states that xml:* namespace attributes (xml:lang, xml:space, etc.) are NOT imported into orphan nodes of a document subset. This is one of the two explicit differences from Inclusive C14N. The `collect_inherited_xml_attrs` function in `src/c14n/serialize.rs` must be skipped entirely for Exclusive mode. RFC 3741 §4.4 places the burden on callers to explicitly include required xml:* attributes in the subset — it does NOT restore the inheritance rule.

Learnt from: CR
Repo: structured-world/xml-sec PR: 0
File: .github/instructions/rust.instructions.md:0-0
Timestamp: 2026-03-17T12:12:28.146Z
Learning: Applies to **/*.rs : Tier 1 - Flag logic bugs: checksum/hash mismatches (computing over wrong byte range, verifying against stale value)

Learnt from: CR
Repo: structured-world/xml-sec PR: 0
File: .github/instructions/code-review.instructions.md:0-0
Timestamp: 2026-03-17T12:12:07.790Z
Learning: Applies to **/*.rs : Corruption/validation tests: tamper the relevant field (e.g., digest value, signature bytes, base64 encoding) and assert the error

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/xmldsig/parse.rs`:
- Around line 625-628: The helper is_xml_whitespace_only is duplicated; extract
it into a shared internal module (e.g., xmldsig::utils or
xmldsig::internal::whitespace) as a single pub(crate) function, replace the
duplicate definitions in the parse and verify modules to call the new shared
function, and remove the redundant implementation so both parse's and verify's
code reference the same is_xml_whitespace_only symbol.

In `@src/xmldsig/verify.rs`:
- Around line 1123-1126: The function is_xml_whitespace_only is duplicated (one
in verify.rs and one in parse.rs); extract it into a single shared internal
utility module (e.g., xml_utils or utils) as a pub(crate) function named
is_xml_whitespace_only, remove the duplicate definitions, and update both
modules (verify.rs and parse.rs) to import and call the shared function; ensure
module visibility and imports are corrected and run tests to verify nothing
breaks.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: bf4a51e6-3bdd-45dc-a152-5b985c143e1a

📥 Commits

Reviewing files that changed from the base of the PR and between 5238c92 and eec05eb.

📒 Files selected for processing (2)
  • src/xmldsig/parse.rs
  • src/xmldsig/verify.rs

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

- gate KeyInfo parsing to resolver/no-explicit-key path and add regression coverage
- centralize XML base64 normalization in whitespace helper and reuse in parse/verify
- clarify public docs that only dsig11:ECKeyValue is supported
@polaz
Copy link
Copy Markdown
Member Author

polaz commented Apr 8, 2026

@coderabbitai re-review

@polaz polaz requested a review from Copilot April 8, 2026 21:31
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 8, 2026

@polaz: Kicking off another full review of PR #47 now. I'll cover all current changes across src/xmldsig/parse.rs (parse_key_info, the KeyInfo/KeyInfoSource/KeyValueInfo/X509DataInfo model, DEREncodedKeyValue base64 path, all internal helpers and tests), src/xmldsig/verify.rs (pipeline wiring, ParseKeyInfo error variant, fully hardened parse_signature_children from commits 2004cf8/5238c92/eec05eb/8f86966), src/xmldsig/whitespace.rs, and src/xmldsig/mod.rs.


🧠 Learnings used
Learnt from: polaz
Repo: structured-world/xml-sec PR: 41
File: tests/donor_full_verification_suite.rs:165-178
Timestamp: 2026-04-04T06:14:34.735Z
Learning: In `tests/donor_full_verification_suite.rs` (xml-sec Rust project), the `Expectation::Skip` arm must exercise skipped vectors through `VerifyContext::new().verify(xml)` (no injected PEM key) so that the real key-resolution path runs, and must assert a concrete blocker outcome per case: `Invalid(KeyNotFound)` for vectors with unresolved KeyValue/X.509 key material, and `ParseSignedInfo(UnsupportedAlgorithm)` for DSA-signature vectors. Accepting any `Err` or non-Valid result as a valid skip is insufficient and can mask stale skip reasons or unrelated failures.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 0
File: :0-0
Timestamp: 2026-03-28T15:50:07.840Z
Learning: In `src/xmldsig/verify.rs` (xml-sec Rust project), `push_normalized_signature_text` strips only the four XML whitespace characters (space 0x20, tab 0x09, CR 0x0D, LF 0x0A) from `SignatureValue` text before base64 decoding, and explicitly rejects any other ASCII whitespace (e.g., form feed 0x0C, vertical tab 0x0B) as `SignatureVerificationPipelineError::SignatureValueBase64(base64::DecodeError::InvalidByte(...))`. The regression test `push_normalized_signature_text_rejects_form_feed` covers the form-feed boundary case.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 0
File: :0-0
Timestamp: 2026-04-08T20:07:42.404Z
Learning: In `src/xmldsig/parse.rs` (xml-sec Rust project), `parse_key_value_dispatch` accepts `ECKeyValue` only when its namespace is `XMLDSIG11_NS` (XMLDSig 1.1). A `ds:`-namespaced `ECKeyValue` (i.e., in the base XMLDSig namespace) falls through to `KeyValueInfo::Unsupported`. The regression test `parse_key_info_marks_ds_namespace_ec_keyvalue_as_unsupported` covers this boundary. Do not flag `ds:ECKeyValue` → Unsupported as a bug; it is the correct fail-safe behavior per the XMLDSig 1.1 spec.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 32
File: src/xmldsig/verify.rs:34-51
Timestamp: 2026-03-28T22:00:29.444Z
Learning: In `src/xmldsig/verify.rs` (xml-sec Rust project), the `VerifyingKey` and `KeyResolver` public traits intentionally omit `Send + Sync` supertraits to preserve lightweight single-threaded implementations. The single-threaded-only constraint is documented on both trait definitions. Do not flag the absence of `Send + Sync` on these traits as a review issue.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 0
File: :0-0
Timestamp: 2026-03-28T18:51:49.919Z
Learning: In `src/xmldsig/parse.rs` (xml-sec Rust project), the private function `n` (formerly `base64_decode_digest`) strips only the four XML whitespace characters (space 0x20, tab 0x09, CR 0x0D, LF 0x0A) from `DigestValue` base64 text before decoding, and returns `ParseError::Base64(format!("invalid XML whitespace U+{:04X} ...", ...))` for any other `is_ascii_whitespace()` character (e.g. form feed 0x0C). Regression tests `n_accepts_xml_whitespace_chars` and `n_rejects_non_xml_ascii_whitespace` cover these cases.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 0
File: :0-0
Timestamp: 2026-03-28T20:47:05.798Z
Learning: In `src/xmldsig/verify.rs` (xml-sec Rust project), the ASCII-whitespace branch in `push_normalized_signature_text` uses `u8::try_from(u32::from(ch)).expect("ASCII whitespace always fits into u8")` for the explicit narrowing conversion, producing `base64::DecodeError::InvalidByte(normalized.len(), invalid_byte)` as the error payload. The conversion is provably infallible because `ch.is_ascii_whitespace()` guarantees the character is in the 0x00–0x7F range.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 12
File: src/c14n/serialize.rs:382-398
Timestamp: 2026-03-16T17:41:41.053Z
Learning: Repo: structured-world/xml-sec (Rust)
Context: src/c14n/serialize.rs::collect_inherited_xml_attrs
Learning: For inclusive C14N subsets, collect_inherited_xml_attrs must stop at the nearest included ancestor (A in set → B out → C in) to avoid inheriting xml:* through an included ancestor. A unit test named `no_inheritance_past_included_ancestor` covers this scenario. Exclusive C14N should skip this inheritance step entirely.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 47
File: src/xmldsig/parse.rs:571-621
Timestamp: 2026-04-08T21:10:12.266Z
Learning: In `src/xmldsig/whitespace.rs` (xml-sec Rust project), the shared helper `normalize_xml_base64_text` strips only the four XML whitespace characters (space 0x20, tab 0x09, CR 0x0D, LF 0x0A) from base64 text and returns a `ParseError::Base64` for any other `is_ascii_whitespace()` character (e.g. form feed 0x0C). It is reused by `base64_decode_digest` (in `parse.rs`), `decode_der_encoded_key_value_base64` (in `parse.rs`), and `push_normalized_signature_text` (in `verify.rs`); each call site applies its own per-call length limits and error mapping on top.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 16
File: tests/fixtures_smoke.rs:104-104
Timestamp: 2026-03-17T15:17:20.125Z
Learning: In structured-world/xml-sec (Rust), `expect()` on `fs::read_to_string()` and other I/O calls is acceptable and idiomatic inside `#[test]` functions. The Tier 2 rule (use `Result` propagation instead of `expect()` on I/O paths) applies only to production code paths, not test code. Repo instructions (line 51) state: "Test code style: Tests prioritize readability and explicitness over DRY."

Learnt from: polaz
Repo: structured-world/xml-sec PR: 6
File: src/c14n/ns_exclusive.rs:81-107
Timestamp: 2026-03-15T19:26:11.246Z
Learning: In the xml-sec project (Rust), roxmltree 0.21 does not expose lexical prefixes from parsed QNames — `ExpandedName` only provides `namespace()` and `name()`. As a result, `visibly_utilized_prefixes()` in `src/c14n/ns_exclusive.rs` and `write_qualified_name()` in `src/c14n/serialize.rs` must use `lookup_prefix(namespace_uri)` for prefix resolution. This is a known limitation: when multiple prefixes bind the same namespace URI, the reverse-mapping is ambiguous. The ambiguity is documented in code comments and tracked as a ROADMAP gap; the edge case is considered rare in SAML/XMLDSig use cases.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 12
File: src/c14n/serialize.rs:218-222
Timestamp: 2026-03-16T18:07:55.027Z
Learning: In the xml-sec project (Rust), RFC 3741 §1.2 (Exclusive XML Canonicalization) explicitly states that xml:* namespace attributes (xml:lang, xml:space, etc.) are NOT imported into orphan nodes of a document subset. This is one of the two explicit differences from Inclusive C14N. The `collect_inherited_xml_attrs` function in `src/c14n/serialize.rs` must be skipped entirely for Exclusive mode. RFC 3741 §4.4 places the burden on callers to explicitly include required xml:* attributes in the subset — it does NOT restore the inheritance rule.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 12
File: src/c14n/xml_base.rs:61-93
Timestamp: 2026-03-16T17:23:26.715Z
Learning: In the xml-sec Rust project, the xml:base URI resolver in `src/c14n/xml_base.rs` correctly uses RFC 3986 §5 for IRI resolution in C14N 1.1 mode, as C14N 1.1 §2.4 explicitly delegates to RFC 3986 §5. When the base URI has no scheme, `parse_base()` returns `None` and the reference is returned as-is — this is the correct fallback behavior. The W3C C14N 1.1 interop test suite notes "modifications to RFC 3986 dot segment processing for C14N11" which may represent edge cases worth verifying against the `remove_dot_segments()` implementation.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 41
File: tests/donor_full_verification_suite.rs:164-168
Timestamp: 2026-04-03T14:47:20.446Z
Learning: In `tests/donor_full_verification_suite.rs` (xml-sec, Rust), using `let _ = read_fixture(...)` to intentionally discard a `String` return value triggers `clippy::let_underscore_drop` under `-D warnings`, because `String` has a non-trivial `Drop` impl. The correct idiom is a plain statement call (e.g., `read_fixture(&root.join(case.xml_path));`) with an inline comment to document intent. Do not suggest `let _ = expr` for types with meaningful `Drop` implementations in this codebase.
✅ Actions performed

Full review triggered.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/xmldsig/parse.rs (1)

539-552: ⚠️ Potential issue | 🟠 Major

Bound DigestValue before decode.

DigestValue still reaches this function at arbitrary length and is only rejected after STANDARD.decode() plus the final output-length check. A hostile signature can therefore force large transient allocations with an oversized <DigestValue>, even though the decoded size is fixed by digest_method. Please mirror the bounded streaming approach used for DEREncodedKeyValue: normalize from the text children, cap the normalized base64 length at digest_method.output_len().div_ceil(3) * 4, and fail before decode.

As per coding guidelines, Tier 1 - Flag logic bugs: missing validation (unchecked index, unvalidated input from network/disk/external source).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/xmldsig/parse.rs` around lines 539 - 552, In base64_decode_digest,
prevent oversized allocations by computing the maximum allowed base64 text
length from the expected decoded size: let cap = ((digest_method.output_len() +
2) / 3) * 4; allocate the normalized buffer with capacity cap (or min(cap,
b64.len())), call normalize_xml_base64_text(b64, &mut cleaned) as before, and
immediately fail with a ParseError::Base64 if cleaned.len() > cap (so the
function returns before calling STANDARD.decode); update the error message to
clearly indicate "DigestValue exceeds maximum allowed base64 length" and
reference function names base64_decode_digest, normalize_xml_base64_text, and
type DigestAlgorithm to locate the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/xmldsig/verify.rs`:
- Around line 557-564: The rustdoc pipeline comment and function docs
incorrectly state that `<KeyInfo>` parsing/validation occurs for all APIs;
update the documentation to reflect that verify_signature_with_pem_key()
unconditionally installs an explicit key (the code path that sets the key in
verify_signature_with_pem_key) and therefore skips parsing/validating
`<KeyInfo>`—remove or mark step 2 as "skipped for verify_signature_with_pem_key"
and add a short note in the verify_signature_with_pem_key() doc comment
clarifying that this API bypasses `<KeyInfo>` processing and uses the provided
PEM key directly.
- Around line 631-635: The code currently unconditionally calls parse_key_info
when ctx.key is None (using ctx.key, signature_children.key_info_node and
parse_key_info), which hard-fails even for resolver-backed verification that
doesn't consume KeyInfo; change this so parsing is opt-in or the parsed
structure is passed into the resolver: either (A) only call parse_key_info when
the selected KeyResolver indicates it will consume document KeyInfo (modify the
resolver selection flow to query a capability flag on KeyResolver), or (B)
change the KeyResolver API to accept an Option<ParsedKeyInfo> (introduce a
ParsedKeyInfo type produced by parse_key_info and thread it into the resolver
call where KeyResolver is invoked around the code that currently uses
KeyResolver at the 903-913 site), and remove the unconditional parse_key_info
call so failures only occur when the resolver actually consumes the parsed
KeyInfo.

---

Outside diff comments:
In `@src/xmldsig/parse.rs`:
- Around line 539-552: In base64_decode_digest, prevent oversized allocations by
computing the maximum allowed base64 text length from the expected decoded size:
let cap = ((digest_method.output_len() + 2) / 3) * 4; allocate the normalized
buffer with capacity cap (or min(cap, b64.len())), call
normalize_xml_base64_text(b64, &mut cleaned) as before, and immediately fail
with a ParseError::Base64 if cleaned.len() > cap (so the function returns before
calling STANDARD.decode); update the error message to clearly indicate
"DigestValue exceeds maximum allowed base64 length" and reference function names
base64_decode_digest, normalize_xml_base64_text, and type DigestAlgorithm to
locate the change.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: d0141ed7-7784-4822-8d09-8d17bd2c6dd7

📥 Commits

Reviewing files that changed from the base of the PR and between dad900e and 3d7b3e0.

📒 Files selected for processing (4)
  • src/xmldsig/mod.rs
  • src/xmldsig/parse.rs
  • src/xmldsig/verify.rs
  • src/xmldsig/whitespace.rs

- update PEM verification docs to match explicit-key KeyInfo behavior
- add KeyResolver capability flag for document KeyInfo consumption
- bound DigestValue base64 length before decode and add regressions
@polaz
Copy link
Copy Markdown
Member Author

polaz commented Apr 8, 2026

@coderabbitai Re: outside-diff finding on src/xmldsig/parse.rs:539-552 (base64_decode_digest) — fixed in commit 3abd4e4. Added pre-decode normalized base64 cap derived from DigestAlgorithm::output_len() and return ParseError::Base64("DigestValue exceeds maximum allowed base64 length") before decode; also added regression test base64_decode_digest_rejects_oversized_base64_before_decode.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 8, 2026

Tip

For best results, initiate chat on the files or code changes.

@polaz: Confirmed — commit 3abd4e4 correctly addresses the gap. Deriving the pre-decode cap from DigestAlgorithm::output_len() (rather than a static constant) ensures the bound tracks the actual algorithm output size, and returning ParseError::Base64("DigestValue exceeds maximum allowed base64 length") before the decode call prevents any over-allocation. The regression test base64_decode_digest_rejects_oversized_base64_before_decode pins that boundary. The concern is fully resolved.


✏️ Learnings added
Learnt from: polaz
URL: https://github.com/structured-world/xml-sec/pull/47

Timestamp: 2026-04-08T21:56:49.678Z
Learning: In `src/xmldsig/parse.rs` (xml-sec Rust project), `base64_decode_digest` enforces a pre-decode normalized base64 length cap derived from `DigestAlgorithm::output_len()` and returns `ParseError::Base64("DigestValue exceeds maximum allowed base64 length")` if the normalized base64 text exceeds that cap before passing it to the decoder. The regression test `base64_decode_digest_rejects_oversized_base64_before_decode` covers this boundary. Do not flag the absence of a pre-decode length check in `base64_decode_digest` — it is present and algorithm-specific.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: polaz
Repo: structured-world/xml-sec PR: 41
File: tests/donor_full_verification_suite.rs:165-178
Timestamp: 2026-04-04T06:14:34.735Z
Learning: In `tests/donor_full_verification_suite.rs` (xml-sec Rust project), the `Expectation::Skip` arm must exercise skipped vectors through `VerifyContext::new().verify(xml)` (no injected PEM key) so that the real key-resolution path runs, and must assert a concrete blocker outcome per case: `Invalid(KeyNotFound)` for vectors with unresolved KeyValue/X.509 key material, and `ParseSignedInfo(UnsupportedAlgorithm)` for DSA-signature vectors. Accepting any `Err` or non-Valid result as a valid skip is insufficient and can mask stale skip reasons or unrelated failures.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 0
File: :0-0
Timestamp: 2026-03-28T15:50:07.840Z
Learning: In `src/xmldsig/verify.rs` (xml-sec Rust project), `push_normalized_signature_text` strips only the four XML whitespace characters (space 0x20, tab 0x09, CR 0x0D, LF 0x0A) from `SignatureValue` text before base64 decoding, and explicitly rejects any other ASCII whitespace (e.g., form feed 0x0C, vertical tab 0x0B) as `SignatureVerificationPipelineError::SignatureValueBase64(base64::DecodeError::InvalidByte(...))`. The regression test `push_normalized_signature_text_rejects_form_feed` covers the form-feed boundary case.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 0
File: :0-0
Timestamp: 2026-04-08T20:07:42.404Z
Learning: In `src/xmldsig/parse.rs` (xml-sec Rust project), `parse_key_value_dispatch` accepts `ECKeyValue` only when its namespace is `XMLDSIG11_NS` (XMLDSig 1.1). A `ds:`-namespaced `ECKeyValue` (i.e., in the base XMLDSig namespace) falls through to `KeyValueInfo::Unsupported`. The regression test `parse_key_info_marks_ds_namespace_ec_keyvalue_as_unsupported` covers this boundary. Do not flag `ds:ECKeyValue` → Unsupported as a bug; it is the correct fail-safe behavior per the XMLDSig 1.1 spec.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 32
File: src/xmldsig/verify.rs:34-51
Timestamp: 2026-03-28T22:00:29.444Z
Learning: In `src/xmldsig/verify.rs` (xml-sec Rust project), the `VerifyingKey` and `KeyResolver` public traits intentionally omit `Send + Sync` supertraits to preserve lightweight single-threaded implementations. The single-threaded-only constraint is documented on both trait definitions. Do not flag the absence of `Send + Sync` on these traits as a review issue.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 0
File: :0-0
Timestamp: 2026-03-28T18:51:49.919Z
Learning: In `src/xmldsig/parse.rs` (xml-sec Rust project), the private function `n` (formerly `base64_decode_digest`) strips only the four XML whitespace characters (space 0x20, tab 0x09, CR 0x0D, LF 0x0A) from `DigestValue` base64 text before decoding, and returns `ParseError::Base64(format!("invalid XML whitespace U+{:04X} ...", ...))` for any other `is_ascii_whitespace()` character (e.g. form feed 0x0C). Regression tests `n_accepts_xml_whitespace_chars` and `n_rejects_non_xml_ascii_whitespace` cover these cases.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 0
File: :0-0
Timestamp: 2026-03-28T20:47:05.798Z
Learning: In `src/xmldsig/verify.rs` (xml-sec Rust project), the ASCII-whitespace branch in `push_normalized_signature_text` uses `u8::try_from(u32::from(ch)).expect("ASCII whitespace always fits into u8")` for the explicit narrowing conversion, producing `base64::DecodeError::InvalidByte(normalized.len(), invalid_byte)` as the error payload. The conversion is provably infallible because `ch.is_ascii_whitespace()` guarantees the character is in the 0x00–0x7F range.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 12
File: src/c14n/serialize.rs:382-398
Timestamp: 2026-03-16T17:41:41.053Z
Learning: Repo: structured-world/xml-sec (Rust)
Context: src/c14n/serialize.rs::collect_inherited_xml_attrs
Learning: For inclusive C14N subsets, collect_inherited_xml_attrs must stop at the nearest included ancestor (A in set → B out → C in) to avoid inheriting xml:* through an included ancestor. A unit test named `no_inheritance_past_included_ancestor` covers this scenario. Exclusive C14N should skip this inheritance step entirely.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 47
File: src/xmldsig/parse.rs:571-621
Timestamp: 2026-04-08T21:10:12.266Z
Learning: In `src/xmldsig/whitespace.rs` (xml-sec Rust project), the shared helper `normalize_xml_base64_text` strips only the four XML whitespace characters (space 0x20, tab 0x09, CR 0x0D, LF 0x0A) from base64 text and returns a `ParseError::Base64` for any other `is_ascii_whitespace()` character (e.g. form feed 0x0C). It is reused by `base64_decode_digest` (in `parse.rs`), `decode_der_encoded_key_value_base64` (in `parse.rs`), and `push_normalized_signature_text` (in `verify.rs`); each call site applies its own per-call length limits and error mapping on top.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 16
File: tests/fixtures_smoke.rs:104-104
Timestamp: 2026-03-17T15:17:20.125Z
Learning: In structured-world/xml-sec (Rust), `expect()` on `fs::read_to_string()` and other I/O calls is acceptable and idiomatic inside `#[test]` functions. The Tier 2 rule (use `Result` propagation instead of `expect()` on I/O paths) applies only to production code paths, not test code. Repo instructions (line 51) state: "Test code style: Tests prioritize readability and explicitness over DRY."

Learnt from: polaz
Repo: structured-world/xml-sec PR: 6
File: src/c14n/ns_exclusive.rs:81-107
Timestamp: 2026-03-15T19:26:11.246Z
Learning: In the xml-sec project (Rust), roxmltree 0.21 does not expose lexical prefixes from parsed QNames — `ExpandedName` only provides `namespace()` and `name()`. As a result, `visibly_utilized_prefixes()` in `src/c14n/ns_exclusive.rs` and `write_qualified_name()` in `src/c14n/serialize.rs` must use `lookup_prefix(namespace_uri)` for prefix resolution. This is a known limitation: when multiple prefixes bind the same namespace URI, the reverse-mapping is ambiguous. The ambiguity is documented in code comments and tracked as a ROADMAP gap; the edge case is considered rare in SAML/XMLDSig use cases.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 12
File: src/c14n/serialize.rs:218-222
Timestamp: 2026-03-16T18:07:55.027Z
Learning: In the xml-sec project (Rust), RFC 3741 §1.2 (Exclusive XML Canonicalization) explicitly states that xml:* namespace attributes (xml:lang, xml:space, etc.) are NOT imported into orphan nodes of a document subset. This is one of the two explicit differences from Inclusive C14N. The `collect_inherited_xml_attrs` function in `src/c14n/serialize.rs` must be skipped entirely for Exclusive mode. RFC 3741 §4.4 places the burden on callers to explicitly include required xml:* attributes in the subset — it does NOT restore the inheritance rule.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 12
File: src/c14n/xml_base.rs:61-93
Timestamp: 2026-03-16T17:23:26.715Z
Learning: In the xml-sec Rust project, the xml:base URI resolver in `src/c14n/xml_base.rs` correctly uses RFC 3986 §5 for IRI resolution in C14N 1.1 mode, as C14N 1.1 §2.4 explicitly delegates to RFC 3986 §5. When the base URI has no scheme, `parse_base()` returns `None` and the reference is returned as-is — this is the correct fallback behavior. The W3C C14N 1.1 interop test suite notes "modifications to RFC 3986 dot segment processing for C14N11" which may represent edge cases worth verifying against the `remove_dot_segments()` implementation.

Learnt from: CR
Repo: structured-world/xml-sec PR: 0
File: .github/instructions/code-review.instructions.md:0-0
Timestamp: 2026-03-17T12:12:07.790Z
Learning: Applies to **/*.rs : Corruption/validation tests: tamper the relevant field (e.g., digest value, signature bytes, base64 encoding) and assert the error

Learnt from: CR
Repo: structured-world/xml-sec PR: 0
File: .github/instructions/rust.instructions.md:0-0
Timestamp: 2026-03-17T12:12:28.146Z
Learning: Applies to **/*.rs : Tier 1 - Flag logic bugs: checksum/hash mismatches (computing over wrong byte range, verifying against stale value)

Learnt from: polaz
Repo: structured-world/xml-sec PR: 41
File: Cargo.toml:20-27
Timestamp: 2026-04-03T13:15:00.557Z
Learning: In `Cargo.toml` of the xml-sec Rust project, `sha1` and `sha2` are intentionally pinned to version `"0.10"` (not `"0.11"`) because `rsa 0.9` and `signature 2` depend on `digest 0.10`. Upgrading to sha1/sha2 0.11 causes a `digest` 0.10 vs 0.11 trait incompatibility that breaks the RSA/ECDSA verification path. Do not flag sha1 or sha2 at version 0.10 as outdated in this codebase.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/xmldsig/parse.rs`:
- Around line 543-556: The DigestValue base64 length check must run while
streaming text children instead of after building the full string to avoid
unbounded allocation; update parse_reference (or add a helper like
decode_digest_value_children) to iterate node.children(), reject element
children, call normalize_xml_base64_text on each text chunk into a single String
buffer preallocated with max_base64_len (computed from
digest_method.output_len().div_ceil(3) * 4), and after each append check
cleaned.len() against max_base64_len returning ParseError::Base64(...)
immediately if exceeded; keep the existing base64 decode and decoded-length
checks afterwards and use the same error messages.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 0fcd043c-a4cd-4924-adf7-27936dcb5e18

📥 Commits

Reviewing files that changed from the base of the PR and between 3d7b3e0 and 3abd4e4.

📒 Files selected for processing (2)
  • src/xmldsig/parse.rs
  • src/xmldsig/verify.rs

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

- decode DigestValue from node children without building an unbounded intermediate string
- enforce normalized base64 cap while streaming DigestValue chunks
- remove expect-based ASCII conversion in whitespace normalizer
@polaz
Copy link
Copy Markdown
Member Author

polaz commented Apr 8, 2026

@coderabbitai re-review

@polaz polaz requested a review from Copilot April 8, 2026 23:15
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 8, 2026

@polaz: Kicking off another full review of PR #47 now. I'll cover all current changes across src/xmldsig/parse.rs (parse_key_info, KeyInfo/KeyInfoSource/KeyValueInfo/X509DataInfo model, DEREncodedKeyValue base64 path, decode_digest_value_children streaming approach, all internal helpers and tests), src/xmldsig/verify.rs (pipeline wiring with consumes_document_key_info() opt-in, ParseKeyInfo error variant, fully hardened parse_signature_children from commits 2004cf8/5238c92/eec05eb/8f86966/3abd4e4), src/xmldsig/whitespace.rs (normalize_xml_base64_text shared helper), and src/xmldsig/mod.rs.


🧠 Learnings used
Learnt from: polaz
Repo: structured-world/xml-sec PR: 41
File: tests/donor_full_verification_suite.rs:165-178
Timestamp: 2026-04-04T06:14:34.735Z
Learning: In `tests/donor_full_verification_suite.rs` (xml-sec Rust project), the `Expectation::Skip` arm must exercise skipped vectors through `VerifyContext::new().verify(xml)` (no injected PEM key) so that the real key-resolution path runs, and must assert a concrete blocker outcome per case: `Invalid(KeyNotFound)` for vectors with unresolved KeyValue/X.509 key material, and `ParseSignedInfo(UnsupportedAlgorithm)` for DSA-signature vectors. Accepting any `Err` or non-Valid result as a valid skip is insufficient and can mask stale skip reasons or unrelated failures.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 0
File: :0-0
Timestamp: 2026-03-28T15:50:07.840Z
Learning: In `src/xmldsig/verify.rs` (xml-sec Rust project), `push_normalized_signature_text` strips only the four XML whitespace characters (space 0x20, tab 0x09, CR 0x0D, LF 0x0A) from `SignatureValue` text before base64 decoding, and explicitly rejects any other ASCII whitespace (e.g., form feed 0x0C, vertical tab 0x0B) as `SignatureVerificationPipelineError::SignatureValueBase64(base64::DecodeError::InvalidByte(...))`. The regression test `push_normalized_signature_text_rejects_form_feed` covers the form-feed boundary case.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 0
File: :0-0
Timestamp: 2026-04-08T20:07:42.404Z
Learning: In `src/xmldsig/parse.rs` (xml-sec Rust project), `parse_key_value_dispatch` accepts `ECKeyValue` only when its namespace is `XMLDSIG11_NS` (XMLDSig 1.1). A `ds:`-namespaced `ECKeyValue` (i.e., in the base XMLDSig namespace) falls through to `KeyValueInfo::Unsupported`. The regression test `parse_key_info_marks_ds_namespace_ec_keyvalue_as_unsupported` covers this boundary. Do not flag `ds:ECKeyValue` → Unsupported as a bug; it is the correct fail-safe behavior per the XMLDSig 1.1 spec.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 32
File: src/xmldsig/verify.rs:34-51
Timestamp: 2026-03-28T22:00:29.444Z
Learning: In `src/xmldsig/verify.rs` (xml-sec Rust project), the `VerifyingKey` and `KeyResolver` public traits intentionally omit `Send + Sync` supertraits to preserve lightweight single-threaded implementations. The single-threaded-only constraint is documented on both trait definitions. Do not flag the absence of `Send + Sync` on these traits as a review issue.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 47
File: src/xmldsig/verify.rs:631-635
Timestamp: 2026-04-08T21:56:43.844Z
Learning: In `src/xmldsig/verify.rs` (xml-sec Rust project), `KeyResolver` has an opt-in capability method `consumes_document_key_info() -> bool` (default `false`). The pipeline only calls `parse_key_info` and propagates `ParseKeyInfo` errors for resolver-backed verification when the active resolver returns `true` from this method. Default/non-consuming resolvers are intentionally shielded from advisory `<KeyInfo>` parse failures to prevent tampering-induced hard failures.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 0
File: :0-0
Timestamp: 2026-03-28T18:51:49.919Z
Learning: In `src/xmldsig/parse.rs` (xml-sec Rust project), the private function `n` (formerly `base64_decode_digest`) strips only the four XML whitespace characters (space 0x20, tab 0x09, CR 0x0D, LF 0x0A) from `DigestValue` base64 text before decoding, and returns `ParseError::Base64(format!("invalid XML whitespace U+{:04X} ...", ...))` for any other `is_ascii_whitespace()` character (e.g. form feed 0x0C). Regression tests `n_accepts_xml_whitespace_chars` and `n_rejects_non_xml_ascii_whitespace` cover these cases.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 0
File: :0-0
Timestamp: 2026-04-08T21:56:49.678Z
Learning: In `src/xmldsig/parse.rs` (xml-sec Rust project), `base64_decode_digest` enforces a pre-decode normalized base64 length cap derived from `DigestAlgorithm::output_len()` and returns `ParseError::Base64("DigestValue exceeds maximum allowed base64 length")` if the normalized base64 text exceeds that cap before passing it to the decoder. The regression test `base64_decode_digest_rejects_oversized_base64_before_decode` covers this boundary. Do not flag the absence of a pre-decode length check in `base64_decode_digest` — it is present and algorithm-specific.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 0
File: :0-0
Timestamp: 2026-03-28T20:47:05.798Z
Learning: In `src/xmldsig/verify.rs` (xml-sec Rust project), the ASCII-whitespace branch in `push_normalized_signature_text` uses `u8::try_from(u32::from(ch)).expect("ASCII whitespace always fits into u8")` for the explicit narrowing conversion, producing `base64::DecodeError::InvalidByte(normalized.len(), invalid_byte)` as the error payload. The conversion is provably infallible because `ch.is_ascii_whitespace()` guarantees the character is in the 0x00–0x7F range.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 12
File: src/c14n/serialize.rs:382-398
Timestamp: 2026-03-16T17:41:41.053Z
Learning: Repo: structured-world/xml-sec (Rust)
Context: src/c14n/serialize.rs::collect_inherited_xml_attrs
Learning: For inclusive C14N subsets, collect_inherited_xml_attrs must stop at the nearest included ancestor (A in set → B out → C in) to avoid inheriting xml:* through an included ancestor. A unit test named `no_inheritance_past_included_ancestor` covers this scenario. Exclusive C14N should skip this inheritance step entirely.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 47
File: src/xmldsig/parse.rs:543-556
Timestamp: 2026-04-08T23:10:56.420Z
Learning: In `src/xmldsig/parse.rs` (xml-sec Rust project), `parse_reference` delegates `<DigestValue>` decoding to `decode_digest_value_children`, which streams over `node.children()`, rejects element children with `ParseError::InvalidStructure`, calls `normalize_xml_base64_text` on each text chunk into a single buffer pre-allocated with `digest_method.output_len().div_ceil(3) * 4`, and enforces the algorithm-derived base64 cap after each append — returning `ParseError::Base64("DigestValue exceeds maximum allowed base64 length")` immediately if exceeded. No full intermediate `digest_b64` string is built before the cap check. The existing `base64_decode_digest` function is still present (and tested) but is no longer the call site for `DigestValue` text collection from XML nodes.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 6
File: src/c14n/ns_exclusive.rs:81-107
Timestamp: 2026-03-15T19:26:11.246Z
Learning: In the xml-sec project (Rust), roxmltree 0.21 does not expose lexical prefixes from parsed QNames — `ExpandedName` only provides `namespace()` and `name()`. As a result, `visibly_utilized_prefixes()` in `src/c14n/ns_exclusive.rs` and `write_qualified_name()` in `src/c14n/serialize.rs` must use `lookup_prefix(namespace_uri)` for prefix resolution. This is a known limitation: when multiple prefixes bind the same namespace URI, the reverse-mapping is ambiguous. The ambiguity is documented in code comments and tracked as a ROADMAP gap; the edge case is considered rare in SAML/XMLDSig use cases.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 12
File: src/c14n/serialize.rs:218-222
Timestamp: 2026-03-16T18:07:55.027Z
Learning: In the xml-sec project (Rust), RFC 3741 §1.2 (Exclusive XML Canonicalization) explicitly states that xml:* namespace attributes (xml:lang, xml:space, etc.) are NOT imported into orphan nodes of a document subset. This is one of the two explicit differences from Inclusive C14N. The `collect_inherited_xml_attrs` function in `src/c14n/serialize.rs` must be skipped entirely for Exclusive mode. RFC 3741 §4.4 places the burden on callers to explicitly include required xml:* attributes in the subset — it does NOT restore the inheritance rule.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 47
File: src/xmldsig/parse.rs:571-621
Timestamp: 2026-04-08T21:10:12.266Z
Learning: In `src/xmldsig/whitespace.rs` (xml-sec Rust project), the shared helper `normalize_xml_base64_text` strips only the four XML whitespace characters (space 0x20, tab 0x09, CR 0x0D, LF 0x0A) from base64 text and returns a `ParseError::Base64` for any other `is_ascii_whitespace()` character (e.g. form feed 0x0C). It is reused by `base64_decode_digest` (in `parse.rs`), `decode_der_encoded_key_value_base64` (in `parse.rs`), and `push_normalized_signature_text` (in `verify.rs`); each call site applies its own per-call length limits and error mapping on top.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 12
File: src/c14n/xml_base.rs:61-93
Timestamp: 2026-03-16T17:23:26.715Z
Learning: In the xml-sec Rust project, the xml:base URI resolver in `src/c14n/xml_base.rs` correctly uses RFC 3986 §5 for IRI resolution in C14N 1.1 mode, as C14N 1.1 §2.4 explicitly delegates to RFC 3986 §5. When the base URI has no scheme, `parse_base()` returns `None` and the reference is returned as-is — this is the correct fallback behavior. The W3C C14N 1.1 interop test suite notes "modifications to RFC 3986 dot segment processing for C14N11" which may represent edge cases worth verifying against the `remove_dot_segments()` implementation.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 41
File: Cargo.toml:20-27
Timestamp: 2026-04-03T13:15:00.557Z
Learning: In `Cargo.toml` of the xml-sec Rust project, `sha1` and `sha2` are intentionally pinned to version `"0.10"` (not `"0.11"`) because `rsa 0.9` and `signature 2` depend on `digest 0.10`. Upgrading to sha1/sha2 0.11 causes a `digest` 0.10 vs 0.11 trait incompatibility that breaks the RSA/ECDSA verification path. Do not flag sha1 or sha2 at version 0.10 as outdated in this codebase.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 16
File: tests/fixtures_smoke.rs:104-104
Timestamp: 2026-03-17T15:17:20.125Z
Learning: In structured-world/xml-sec (Rust), `expect()` on `fs::read_to_string()` and other I/O calls is acceptable and idiomatic inside `#[test]` functions. The Tier 2 rule (use `Result` propagation instead of `expect()` on I/O paths) applies only to production code paths, not test code. Repo instructions (line 51) state: "Test code style: Tests prioritize readability and explicitness over DRY."

Learnt from: CR
Repo: structured-world/xml-sec PR: 0
File: .github/instructions/rust.instructions.md:0-0
Timestamp: 2026-03-17T12:12:28.146Z
Learning: Applies to **/*.rs : Tier 1 - Flag logic bugs: checksum/hash mismatches (computing over wrong byte range, verifying against stale value)

Learnt from: CR
Repo: structured-world/xml-sec PR: 0
File: .github/instructions/rust.instructions.md:0-0
Timestamp: 2026-03-17T12:12:28.146Z
Learning: Applies to **/*.rs : Tier 2 - Flag safety issues: constant-time comparison must be used for cryptographic MACs/checksums

Learnt from: CR
Repo: structured-world/xml-sec PR: 0
File: .github/instructions/code-review.instructions.md:0-0
Timestamp: 2026-03-17T12:12:07.790Z
Learning: Applies to **/*.rs : Corruption/validation tests: tamper the relevant field (e.g., digest value, signature bytes, base64 encoding) and assert the error
✅ Actions performed

Full review triggered.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

@polaz
Copy link
Copy Markdown
Member Author

polaz commented Apr 9, 2026

@coderabbitai re-review

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 9, 2026

@polaz: Kicking off another full review of PR #47 now. I'll cover all current changes across src/xmldsig/parse.rs (parse_key_info, the KeyInfo/KeyInfoSource/KeyValueInfo/X509DataInfo model, DEREncodedKeyValue base64 path, decode_digest_value_children streaming approach, all internal helpers and tests), src/xmldsig/verify.rs (pipeline wiring with consumes_document_key_info() opt-in, ParseKeyInfo error variant, fully hardened parse_signature_children from commits 2004cf8/5238c92/eec05eb/8f86966/3abd4e4), src/xmldsig/whitespace.rs (normalize_xml_base64_text shared helper), and src/xmldsig/mod.rs.


🧠 Learnings used
Learnt from: polaz
Repo: structured-world/xml-sec PR: 41
File: tests/donor_full_verification_suite.rs:165-178
Timestamp: 2026-04-04T06:14:34.735Z
Learning: In `tests/donor_full_verification_suite.rs` (xml-sec Rust project), the `Expectation::Skip` arm must exercise skipped vectors through `VerifyContext::new().verify(xml)` (no injected PEM key) so that the real key-resolution path runs, and must assert a concrete blocker outcome per case: `Invalid(KeyNotFound)` for vectors with unresolved KeyValue/X.509 key material, and `ParseSignedInfo(UnsupportedAlgorithm)` for DSA-signature vectors. Accepting any `Err` or non-Valid result as a valid skip is insufficient and can mask stale skip reasons or unrelated failures.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 0
File: :0-0
Timestamp: 2026-03-28T15:50:07.840Z
Learning: In `src/xmldsig/verify.rs` (xml-sec Rust project), `push_normalized_signature_text` strips only the four XML whitespace characters (space 0x20, tab 0x09, CR 0x0D, LF 0x0A) from `SignatureValue` text before base64 decoding, and explicitly rejects any other ASCII whitespace (e.g., form feed 0x0C, vertical tab 0x0B) as `SignatureVerificationPipelineError::SignatureValueBase64(base64::DecodeError::InvalidByte(...))`. The regression test `push_normalized_signature_text_rejects_form_feed` covers the form-feed boundary case.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 0
File: :0-0
Timestamp: 2026-04-08T20:07:42.404Z
Learning: In `src/xmldsig/parse.rs` (xml-sec Rust project), `parse_key_value_dispatch` accepts `ECKeyValue` only when its namespace is `XMLDSIG11_NS` (XMLDSig 1.1). A `ds:`-namespaced `ECKeyValue` (i.e., in the base XMLDSig namespace) falls through to `KeyValueInfo::Unsupported`. The regression test `parse_key_info_marks_ds_namespace_ec_keyvalue_as_unsupported` covers this boundary. Do not flag `ds:ECKeyValue` → Unsupported as a bug; it is the correct fail-safe behavior per the XMLDSig 1.1 spec.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 32
File: src/xmldsig/verify.rs:34-51
Timestamp: 2026-03-28T22:00:29.444Z
Learning: In `src/xmldsig/verify.rs` (xml-sec Rust project), the `VerifyingKey` and `KeyResolver` public traits intentionally omit `Send + Sync` supertraits to preserve lightweight single-threaded implementations. The single-threaded-only constraint is documented on both trait definitions. Do not flag the absence of `Send + Sync` on these traits as a review issue.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 47
File: src/xmldsig/verify.rs:631-635
Timestamp: 2026-04-08T21:56:43.844Z
Learning: In `src/xmldsig/verify.rs` (xml-sec Rust project), `KeyResolver` has an opt-in capability method `consumes_document_key_info() -> bool` (default `false`). The pipeline only calls `parse_key_info` and propagates `ParseKeyInfo` errors for resolver-backed verification when the active resolver returns `true` from this method. Default/non-consuming resolvers are intentionally shielded from advisory `<KeyInfo>` parse failures to prevent tampering-induced hard failures.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 0
File: :0-0
Timestamp: 2026-03-28T18:51:49.919Z
Learning: In `src/xmldsig/parse.rs` (xml-sec Rust project), the private function `n` (formerly `base64_decode_digest`) strips only the four XML whitespace characters (space 0x20, tab 0x09, CR 0x0D, LF 0x0A) from `DigestValue` base64 text before decoding, and returns `ParseError::Base64(format!("invalid XML whitespace U+{:04X} ...", ...))` for any other `is_ascii_whitespace()` character (e.g. form feed 0x0C). Regression tests `n_accepts_xml_whitespace_chars` and `n_rejects_non_xml_ascii_whitespace` cover these cases.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 0
File: :0-0
Timestamp: 2026-04-08T21:56:49.678Z
Learning: In `src/xmldsig/parse.rs` (xml-sec Rust project), `base64_decode_digest` enforces a pre-decode normalized base64 length cap derived from `DigestAlgorithm::output_len()` and returns `ParseError::Base64("DigestValue exceeds maximum allowed base64 length")` if the normalized base64 text exceeds that cap before passing it to the decoder. The regression test `base64_decode_digest_rejects_oversized_base64_before_decode` covers this boundary. Do not flag the absence of a pre-decode length check in `base64_decode_digest` — it is present and algorithm-specific.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 0
File: :0-0
Timestamp: 2026-03-28T20:47:05.798Z
Learning: In `src/xmldsig/verify.rs` (xml-sec Rust project), the ASCII-whitespace branch in `push_normalized_signature_text` uses `u8::try_from(u32::from(ch)).expect("ASCII whitespace always fits into u8")` for the explicit narrowing conversion, producing `base64::DecodeError::InvalidByte(normalized.len(), invalid_byte)` as the error payload. The conversion is provably infallible because `ch.is_ascii_whitespace()` guarantees the character is in the 0x00–0x7F range.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 12
File: src/c14n/serialize.rs:382-398
Timestamp: 2026-03-16T17:41:41.053Z
Learning: Repo: structured-world/xml-sec (Rust)
Context: src/c14n/serialize.rs::collect_inherited_xml_attrs
Learning: For inclusive C14N subsets, collect_inherited_xml_attrs must stop at the nearest included ancestor (A in set → B out → C in) to avoid inheriting xml:* through an included ancestor. A unit test named `no_inheritance_past_included_ancestor` covers this scenario. Exclusive C14N should skip this inheritance step entirely.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 47
File: src/xmldsig/parse.rs:543-556
Timestamp: 2026-04-08T23:10:56.420Z
Learning: In `src/xmldsig/parse.rs` (xml-sec Rust project), `parse_reference` delegates `<DigestValue>` decoding to `decode_digest_value_children`, which streams over `node.children()`, rejects element children with `ParseError::InvalidStructure`, calls `normalize_xml_base64_text` on each text chunk into a single buffer pre-allocated with `digest_method.output_len().div_ceil(3) * 4`, and enforces the algorithm-derived base64 cap after each append — returning `ParseError::Base64("DigestValue exceeds maximum allowed base64 length")` immediately if exceeded. No full intermediate `digest_b64` string is built before the cap check. The existing `base64_decode_digest` function is still present (and tested) but is no longer the call site for `DigestValue` text collection from XML nodes.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 6
File: src/c14n/ns_exclusive.rs:81-107
Timestamp: 2026-03-15T19:26:11.246Z
Learning: In the xml-sec project (Rust), roxmltree 0.21 does not expose lexical prefixes from parsed QNames — `ExpandedName` only provides `namespace()` and `name()`. As a result, `visibly_utilized_prefixes()` in `src/c14n/ns_exclusive.rs` and `write_qualified_name()` in `src/c14n/serialize.rs` must use `lookup_prefix(namespace_uri)` for prefix resolution. This is a known limitation: when multiple prefixes bind the same namespace URI, the reverse-mapping is ambiguous. The ambiguity is documented in code comments and tracked as a ROADMAP gap; the edge case is considered rare in SAML/XMLDSig use cases.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 12
File: src/c14n/serialize.rs:218-222
Timestamp: 2026-03-16T18:07:55.027Z
Learning: In the xml-sec project (Rust), RFC 3741 §1.2 (Exclusive XML Canonicalization) explicitly states that xml:* namespace attributes (xml:lang, xml:space, etc.) are NOT imported into orphan nodes of a document subset. This is one of the two explicit differences from Inclusive C14N. The `collect_inherited_xml_attrs` function in `src/c14n/serialize.rs` must be skipped entirely for Exclusive mode. RFC 3741 §4.4 places the burden on callers to explicitly include required xml:* attributes in the subset — it does NOT restore the inheritance rule.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 47
File: src/xmldsig/parse.rs:571-621
Timestamp: 2026-04-08T21:10:12.266Z
Learning: In `src/xmldsig/whitespace.rs` (xml-sec Rust project), the shared helper `normalize_xml_base64_text` strips only the four XML whitespace characters (space 0x20, tab 0x09, CR 0x0D, LF 0x0A) from base64 text and returns a `ParseError::Base64` for any other `is_ascii_whitespace()` character (e.g. form feed 0x0C). It is reused by `base64_decode_digest` (in `parse.rs`), `decode_der_encoded_key_value_base64` (in `parse.rs`), and `push_normalized_signature_text` (in `verify.rs`); each call site applies its own per-call length limits and error mapping on top.

Learnt from: polaz
Repo: structured-world/xml-sec PR: 12
File: src/c14n/xml_base.rs:61-93
Timestamp: 2026-03-16T17:23:26.715Z
Learning: In the xml-sec Rust project, the xml:base URI resolver in `src/c14n/xml_base.rs` correctly uses RFC 3986 §5 for IRI resolution in C14N 1.1 mode, as C14N 1.1 §2.4 explicitly delegates to RFC 3986 §5. When the base URI has no scheme, `parse_base()` returns `None` and the reference is returned as-is — this is the correct fallback behavior. The W3C C14N 1.1 interop test suite notes "modifications to RFC 3986 dot segment processing for C14N11" which may represent edge cases worth verifying against the `remove_dot_segments()` implementation.
✅ Actions performed

Full review triggered.

@polaz polaz merged commit f80a509 into main Apr 9, 2026
13 checks passed
@polaz polaz deleted the feat/#46-keyinfo-dispatch branch April 9, 2026 06:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

P2-001: KeyInfo parsing dispatch for XMLDSig verify

2 participants